PR: Fix folding and make some performance improvements (Editor) #21669

ccordoba12 · 2024-01-02T16:10:15Z

Description of Changes

For folding

Fix removing a selection that starts or ends in a folded line (this was totally broken).
Remove folded regions when pressing Delete or Backspace in a folded line. I took this idea from VSCode.
Update folding decorations in the visible buffer. Before we were doing it in the visible region, but that prevents to quickly show folded regions when scrolling a file.
Update folding in a thread. Although this doesn't avoid computing folding in the main thread (due to the GIL), it'll help to do it in an external process (I'll do that in a follow up PR to not make this one bigger).
Check that folded regions in the visible buffer are updated correctly by UI inspection. We need to do this because the algorithm to update folding fails sometimes (we'll be able to improve that algorithm after folding is computed in an external process).
Make folded lines appear as read-only to users, i.e. users won't be able to edit them. They'll only be able to delete them by pressing the Delete or Backspace keys.
Expand test suite to cover the new fixes.

Other improvements

Update scroll flags in a thread.
Prevent updating scroll flags if the cursor is being moved, e.g. when typing.
Avoid computing some unnecessary operations for code snippets.
Fix reporting errors that come from PyLSP (this was broken too).

Visual changes

Use chevron icons in the folding panel and move it to be next to the code

The last part preserves the UI we have in Spyder 5, which I think is the right one.

Before	After

Preserve syntax highlighting for folded lines and extend their background color to the full width of the editor

I took this idea from VSCode.

Before	After

Fix style of the folding panel in the light theme

Before	After

Issue(s) Resolved

Part of #21637.

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: @ccordoba12

pep8speaks · 2024-01-02T16:10:35Z

Hello @ccordoba12! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file spyder/plugins/editor/panels/scrollflag.py:

Line 133:73: W291 trailing whitespace

In the file spyder/utils/icon_manager.py:

Line 349:80: E501 line too long (95 > 79 characters)
Line 350:80: E501 line too long (94 > 79 characters)

Comment last updated at 2024-02-07 18:19:43 UTC

jitseniesen

Thanks for keeping the commit history so clean. It meant that I could fairly easily steps through the commit instead of viewing all the hundreds of lines at once.

I have only a couple of minor comments, some general, some specific to the code. I'm happy for you to merge after you have had a look at what I wrote.

Pressing Tab on a folded line indents the whole folded region (this is fine), but it also selects the folded region (this is a bit weird). Pressing Shift+Tab un-indents only the first line of the folded region. I don't think this is intentional. It's a bit of a boundary case, though, so not that important.
The folding indicator no longer changes colour when you hover above them. That's not a problem as far as I am concerned.
Remove folded regions when pressing Delete or Backspace in a folded line. I took this idea from VSCode.

This means that the editor acts as if the whole folded region is selected if you are on a folded line. The user can't edit a folded line (unless they unfold), they can only delete the whole folded region. I find that surprising, which is not good. On the other hand, I never use folding so I don't have an intuition, and if VSCode does this then the UX is probably not too bad. Thus I'm fine with this.
Preserve syntax highlighting for folded lines and extend their background color to the full width of the editor

The disadvantage is that before it was obvious when a region was folded, but now it is less so. I guess the chevron should be enough of a sign for users.

spyder/plugins/editor/panels/scrollflag.py

spyder/plugins/editor/utils/decoration.py

spyder/plugins/editor/panels/codefolding.py

This prevents freezes in the interface, which was happening before because half of that computation was done on the main thread.

…esponses This fixes showing LSP errors on our side, which was broken before (who knows since when).

This prevents freezing the interface by running that computation on the main thread.

Now we use a timer to avoid too many calls to that method.

… there is an active snippet

This allows to perform that task on the thread we have for folding, instead of doing it on the main one.

Also, remove some unused attributes

This was totally broken.

- We need to do this because those operations could have removed some block decorations. - This also allows us to call the _on_key_pressed and _expand_selection methods only when they are needed.

This clears some decorations that are left in the editor when undoing the removal of unfolded regions.

- This gives more flexibility regarding when to paint those regions. - It also prevents some glitches in the interface when undoing the removal of folded regions.

This prevents not painting folded decorations on lines close to the visible region.

Also, highlight folded blocks when updating decorations in the editor. This is necessary to decorate blocks not detected as folded by merge_folding, which are outside of the editor's buffer region.

We're not using them, so that code is not really necessary.

This also fixes the problem that uncollapsed icons were barely visible on the light theme when hovering the folding panel because they had a yellow color.

They are the same ones used by VSCode and easier to tell apart.

- Correctly paint the panel on hover for the light theme. - Remove foreground color and outline for folded blocks and paint their decorations in full width instead.

Also, use qdebounce instead of a timer to simplify code a bit.

This way we avoid to compute one extra `for` loop while updating folding.

Also, remove an unnecessary block comment.

That avoids an extra operating to get a reference for it in several places.

ccordoba12 · 2024-02-06T16:10:56Z

Thanks for the review @jitseniesen! About your comments:

Pressing Tab on a folded line indents the whole folded region (this is fine), but it also selects the folded region (this is a bit weird). Pressing Shift+Tab un-indents only the first line of the folded region. I don't think this is intentional. It's a bit of a boundary case, though, so not that important.

I was not aware of that and it was not intended. So, I made that pressing Tab and Shift+Tab in a folded line do nothing.

The user can't edit a folded line (unless they unfold), they can only delete the whole folded region. I find that surprising, which is not good.

The problem is that if we allow editing folded lines, the editor could be left in an inconsistent state in the sense that the new results coming from the LSP could not include the current line as folded. That's why folded lines are like read-only ones.

On the other hand, I never use folding so I don't have an intuition, and if VSCode does this then the UX is probably not too bad. Thus I'm fine with this.

Ok, thanks for the remark.

The disadvantage is that before it was obvious when a region was folded, but now it is less so. I guess the chevron should be enough of a sign for users.

A point in favor of that change is that folded lines are now highlighted to the editor's full width. So you can spot them while moving up and down more easily. And this is also what VSCode does, i.e. it preserves syntax highlighting in folded lines.

jitseniesen · 2024-02-06T16:26:48Z

The user can't edit a folded line (unless they unfold), they can only delete the whole folded region. I find that surprising, which is not good.

The problem is that if we allow editing folded lines, the editor could be left in an inconsistent state in the sense that the new results coming from the LSP could not include the current line as folded. That's why folded lines are like read-only ones.

I mean, if I move the cursor to a folded line and then I press for instance x, then the whole folded region is deleted and the letter x is added to the next line. If you want folded lines to be like read-only ones (which I'd agree with), then I'd expect that pressing x on a folded line does nothing.

Also, make others run on Windows

Co-authored-by: Jitse Niesen <[email protected]>

ccordoba12 · 2024-02-06T16:52:16Z

I mean, if I move the cursor to a folded line and then I press for instance x, then the whole folded region is deleted and the letter x is added to the next line. If you want folded lines to be like read-only ones (which I'd agree with), then I'd expect that pressing x on a folded line does nothing.

Ok, that's a good point and I also agree with it, so I'll implement it before merging this PR.

That will make appear as read-only to users.

ccordoba12 · 2024-02-07T19:17:58Z

I think this one is finally ready. @jitseniesen, if you find additional issues with folding please open a new issue and I'll address them in a follow-up PR.

ccordoba12 added type:Bug type:Enhancement component:Editor labels Jan 2, 2024

ccordoba12 added this to the v6.0beta1 milestone Jan 2, 2024

ccordoba12 requested a review from dalthviz January 2, 2024 16:10

ccordoba12 self-assigned this Jan 2, 2024

ccordoba12 force-pushed the fix-editor-freezes branch from 91360f4 to beb2c1a Compare January 2, 2024 16:14

ccordoba12 added component:Code Completion and linting User Interface (UI) labels Jan 2, 2024

ccordoba12 force-pushed the fix-editor-freezes branch 2 times, most recently from d9e7c03 to 10ebc32 Compare January 3, 2024 16:07

rear1019 mentioned this pull request Jan 9, 2024

editor: Fix indention guides and code folding for cloned editors #21033

Draft

3 tasks

ccordoba12 force-pushed the fix-editor-freezes branch from 10ebc32 to ba770b3 Compare January 17, 2024 23:30

ccordoba12 requested review from jitseniesen and removed request for dalthviz January 18, 2024 20:27

jitseniesen approved these changes Jan 21, 2024

View reviewed changes

ccordoba12 added 13 commits February 2, 2024 19:06

Mixins: Fix header comments so they appear in Spyder's outline

3aa18cf

Mixins: Rewrite get_text_region to be thread safe

ae2c4f4

Editor: Completely update folding info on a thread

32beb40

This prevents freezes in the interface, which was happening before because half of that computation was done on the main thread.

Editor: Improve how we manage errors that appear while handling LSP r…

20eea96

…esponses This fixes showing LSP errors on our side, which was broken before (who knows since when).

Scrollflag: Update flags shown in ScrollFlagArea using a thread

112de64

This prevents freezing the interface by running that computation on the main thread.

Scrollflag: Add logging message when updating scroll area flags

c264c06

Code snippets: Delay calling cursor_changed a bit

c73deb9

Now we use a timer to avoid too many calls to that method.

Code snippets: Only call code that involves _find_node_by_position if…

83971de

… there is an active snippet

Editor: Extract folding ranges info directly on it

dc4b2e0

This allows to perform that task on the thread we have for folding, instead of doing it on the main one.

Editor: Remove unnecesary log message

33e6630

Folding: Move init to be at the beginning

87492b8

Also, remove some unused attributes

Folding: Fix removing folded regions

f32fc9c

This was totally broken.

Folding: Update block decorations when removing or replacing text

df623ce

- We need to do this because those operations could have removed some block decorations. - This also allows us to call the _on_key_pressed and _expand_selection methods only when they are needed.

ccordoba12 added 13 commits February 2, 2024 19:06

Folding: Clear block decorations when new info arrives from the LSP

fa8b4ca

This clears some decorations that are left in the editor when undoing the removal of unfolded regions.

Folding: Don't paint decorations for folded regions on its paintEvent

6a5bcc1

- This gives more flexibility regarding when to paint those regions. - It also prevents some glitches in the interface when undoing the removal of folded regions.

Folding: Highlight folded regions on the editor's visible buffer

0b05a2b

This prevents not painting folded decorations on lines close to the visible region.

Folding: Check if blocks are folded by UI inspection

7dfa160

Also, highlight folded blocks when updating decorations in the editor. This is necessary to decorate blocks not detected as folded by merge_folding, which are outside of the editor's buffer region.

Folding: Remove code to use native icons

79a96e4

We're not using them, so that code is not really necessary.

Folding: Simplify the way we paint collapsed/uncollapsed icons

0f6a280

This also fixes the problem that uncollapsed icons were barely visible on the light theme when hovering the folding panel because they had a yellow color.

Folding: Use chevron icons for folding

c9bd2a0

They are the same ones used by VSCode and easier to tell apart.

Folding: A couple of style improvements

5c50c2b

- Correctly paint the panel on hover for the light theme. - Remove foreground color and outline for folded blocks and paint their decorations in full width instead.

Scrollflag: Delay updating it while users are moving the cursor

9b43b71

Also, use qdebounce instead of a timer to simplify code a bit.

Folding: Pass ranges that come from the LSP directly to merge_folding

fb3f111

This way we avoid to compute one extra `for` loop while updating folding.

Testing: Expand editor folding tests

6d625eb

Also, remove an unnecessary block comment.

Editor: Use an attribute in CodeEditor for the folding panel

66ad46d

That avoids an extra operating to get a reference for it in several places.

Editor: Restore Spyder 5 order of left side panels

1c5bace

ccordoba12 force-pushed the fix-editor-freezes branch 2 times, most recently from dc4fea8 to 5879b6d Compare February 4, 2024 00:30

ccordoba12 mentioned this pull request Feb 6, 2024

PR: Update for new menuinst and friends and new Spyder feedstock (Installers) #21647

Merged

4 tasks

ccordoba12 force-pushed the fix-editor-freezes branch from 304a694 to 8004add Compare February 6, 2024 15:54

ccordoba12 and others added 3 commits February 6, 2024 11:43

Testing: Fix failing tests

1a0153a

Also, make others run on Windows

Suggestions from code review

cc5a0aa

Co-authored-by: Jitse Niesen <[email protected]>

Decorations: Remove unneeded check for default key value in add/remove

a408d3f

Folding: Prevent Tab/Shift+Tab to modify the contents of folded lines

b19adfe

ccordoba12 force-pushed the fix-editor-freezes branch from 8004add to 2fdd883 Compare February 7, 2024 18:12

Folding: Prevent typing in folded lines to change them

ed35459

That will make appear as read-only to users.

ccordoba12 force-pushed the fix-editor-freezes branch from 2fdd883 to ed35459 Compare February 7, 2024 18:19

ccordoba12 merged commit 55e92d7 into spyder-ide:master Feb 7, 2024
14 checks passed

ccordoba12 deleted the fix-editor-freezes branch February 7, 2024 19:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR: Fix folding and make some performance improvements (Editor) #21669

PR: Fix folding and make some performance improvements (Editor) #21669

ccordoba12 commented Jan 2, 2024 •

edited

Loading

pep8speaks commented Jan 2, 2024 •

edited

Loading

jitseniesen left a comment

ccordoba12 commented Feb 6, 2024

jitseniesen commented Feb 6, 2024

ccordoba12 commented Feb 6, 2024 •

edited

Loading

ccordoba12 commented Feb 7, 2024

PR: Fix folding and make some performance improvements (Editor) #21669

PR: Fix folding and make some performance improvements (Editor) #21669

Conversation

ccordoba12 commented Jan 2, 2024 • edited Loading

Description of Changes

For folding

Other improvements

Visual changes

Issue(s) Resolved

Affirmation

pep8speaks commented Jan 2, 2024 • edited Loading

Comment last updated at 2024-02-07 18:19:43 UTC

jitseniesen left a comment

Choose a reason for hiding this comment

ccordoba12 commented Feb 6, 2024

jitseniesen commented Feb 6, 2024

ccordoba12 commented Feb 6, 2024 • edited Loading

ccordoba12 commented Feb 7, 2024

ccordoba12 commented Jan 2, 2024 •

edited

Loading

pep8speaks commented Jan 2, 2024 •

edited

Loading

ccordoba12 commented Feb 6, 2024 •

edited

Loading